Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failed tests on 32-bit OS #1540

Merged
merged 1 commit into from
Aug 2, 2024
Merged

Conversation

roger2hk
Copy link
Contributor

Fixes #1539.

Checklist

@roger2hk roger2hk requested review from mhutchinson and AlCutter July 27, 2024 21:09
@roger2hk roger2hk requested a review from a team as a code owner July 27, 2024 21:09
@roger2hk
Copy link
Contributor Author

@daviddrysdale We would like to avoid any change in the forked x509 package. Should I change the type from int to int64 or skip the test in 32-bit OS?

@roger2hk
Copy link
Contributor Author

roger2hk commented Aug 1, 2024

@AlCutter PTAL. The x509/rpki_test.go will only be tested in the listed 64-bit $GOARCH.

@AlCutter
Copy link
Member

AlCutter commented Aug 2, 2024

It feels icky, but I guess it'll unblock the issue.

For posterity, here's the info I dug up which is related to the issue:

Seems like AS numbers are [defined to be unsigned 32bit values](https://www.arin.net/resources/guide/asn/#:~:text=This%20format%20provides%20for%2065%2C536,ASNs%20(0%20to%204294967295), but the ASN.1 is INTEGER used to stored them in X.509 is signed and of arbitrary size.

I guess this means that the rpki stuff as written is also latently semi-broken on 32bit archs - if it encounters a valid cert with ASIDRange.Max > 231-1 it'll just error out when trying to parse it since the target type isn't large enough to contain it.

Another, although potentially breaking, option might be to make ASIDRange.Min and ASIDRange.Max be int64 (See https://pkg.go.dev/encoding/asn1#Unmarshal). Perhaps we can come back and do that if we find out that using this server-side code on 32bit systems is actually a thing.

@roger2hk roger2hk merged commit f5239c0 into google:master Aug 2, 2024
6 checks passed
@roger2hk
Copy link
Contributor Author

roger2hk commented Aug 2, 2024

It feels icky, but I guess it'll unblock the issue.

For posterity, here's the info I dug up which is related to the issue:

Seems like AS numbers are [defined to be unsigned 32bit values](https://www.arin.net/resources/guide/asn/#:~:text=This%20format%20provides%20for%2065%2C536,ASNs%20(0%20to%204294967295), but the ASN.1 is INTEGER used to stored them in X.509 is signed and of arbitrary size.

I guess this means that the rpki stuff as written is also latently semi-broken on 32bit archs - if it encounters a valid cert with ASIDRange.Max > 231-1 it'll just error out when trying to parse it since the target type isn't large enough to contain it.

Another, although potentially breaking, option might be to make ASIDRange.Min and ASIDRange.Max be int64 (See https://pkg.go.dev/encoding/asn1#Unmarshal). Perhaps we can come back and do that if we find out that using this server-side code on 32bit systems is actually a thing.

@AlCutter This is a really good write-up of the issue. What if we put this into the rpki package comment?

@roger2hk roger2hk deleted the fix-32bit-tests branch August 21, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure on 32bit systems
2 participants